Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show improved Dax onboarding on New Tab Page #3203

Merged
merged 2 commits into from
Aug 8, 2024

Conversation

dus7
Copy link
Contributor

@dus7 dus7 commented Aug 7, 2024

Task/Issue URL: https://app.asana.com/0/72649045549333/1207981662892269/f
Tech Design URL:
CC: @alessandroboron @SabrinaTardio

Description:

Integration of the new (improved) onboarding with New Tab Page. Previous Dax onboarding is not integrated since it will be removed after the experiment. NTP will start to roll out after the onboarding experiment is concluded.

Steps to test this PR:

  1. Edit DefaultVariantManager so .newOnboardingIntro is always enabled.
  2. Edit NewTabPageManager.isNewTabPageSectionsEnabled so it returns true.
  3. Do a clean install and go through onboarding, verify if it looks as expected.

Definition of Done (Internal Only):

Copy Testing:

  • Use of correct apostrophes in new copy, ie rather than '

Orientation Testing:

  • Portrait
  • Landscape

Device Testing:

  • iPhone SE (1st Gen)
  • iPhone 8
  • iPhone X
  • iPhone 14 Pro
  • iPad

OS Testing:

  • iOS 15
  • iOS 16
  • iOS 17

Theme Testing:

  • Light theme
  • Dark theme

Internal references:

Software Engineering Expectations
Technical Design Template

@dus7 dus7 force-pushed the mariusz/ntp-integrate-onboarding branch from 6e29ee4 to 2edc4a9 Compare August 7, 2024 16:10
@dus7 dus7 force-pushed the mariusz/ntp-integrate-onboarding branch from 2edc4a9 to 8a14f59 Compare August 7, 2024 16:13
@dus7 dus7 marked this pull request as ready for review August 7, 2024 16:14
@dus7 dus7 requested review from alessandroboron and brindy August 7, 2024 16:14
Comment on lines +202 to +230
func showNextDaxDialogNew(dialogProvider: NewTabDialogSpecProvider, factory: any NewTabDaxDialogProvider) {
dismissHostingController(didFinishNTPOnboarding: false)

guard let spec = dialogProvider.nextHomeScreenMessageNew() else { return }

let onDismiss = {
dialogProvider.dismiss()
self.dismissHostingController(didFinishNTPOnboarding: true)
}
let daxDialogView = AnyView(factory.createDaxDialog(for: spec, onDismiss: onDismiss))
let hostingController = UIHostingController(rootView: daxDialogView)
self.hostingController = hostingController

hostingController.view.backgroundColor = .clear
addChild(hostingController)
view.addSubview(hostingController.view)
hostingController.view.translatesAutoresizingMaskIntoConstraints = false

NSLayoutConstraint.activate([
hostingController.view.topAnchor.constraint(equalTo: view.topAnchor),
hostingController.view.leadingAnchor.constraint(equalTo: view.leadingAnchor),
hostingController.view.trailingAnchor.constraint(equalTo: view.trailingAnchor),
hostingController.view.bottomAnchor.constraint(equalTo: view.bottomAnchor)
])

hostingController.didMove(toParent: self)

newTabPageModel.startOnboarding()
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copied the code from HomeViewController+DaxDialogs, but included addChild(hostingController) which was missing in the original.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Included this call in the HomeViewController code (cd424a9).

Copy link
Contributor

@alessandroboron alessandroboron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works as expected! As we will be removing HomeViewController altogether it’s fine if we have temporary duplicate code to initialise the HostingController for the dax dialogs view. Thanks @dus7!

@dus7 dus7 merged commit e561262 into main Aug 8, 2024
13 checks passed
@dus7 dus7 deleted the mariusz/ntp-integrate-onboarding branch August 8, 2024 11:07
samsymons added a commit that referenced this pull request Aug 12, 2024
# By Sam Symons (3) and others
# Via Chris Brind (2) and others
* main:
  DuckPlayer contingency messages pixels (#3220)
  New autofill onboarding for existing DDG users (#3170)
  Add translations for DuckPlayer contingency messge (#3215)
  Fix PrivacyProDataReporter crash due to premature secure vault use (#3217)
  Bump rexml from 3.2.9 to 3.3.3 (#3175)
  Bump submodules/privacy-reference-tests from `a603ff9` to `afb4f61` (#3165)
  [DuckPlayer] 16. Localization Updates (#3200)
  Release 7.132.0-3 (#3213)
  cherry pick temporary fix for https://app.asana.com/0/414235014887631/1207990702991361/f
  fix content inset when keyboard is visible (#3207)
  Ship review feedback for contingency message (#3209)
  Release 7.132.0-2 (#3211)
  Restore attribution flag (#3197)
  Restore attribution flag (#3197)
  Release 7.132.0-1 (#3208)
  Break DuckPlayer ref cycle (#3206)
  Show improved Dax onboarding on New Tab Page (#3203)

# Conflicts:
#	DuckDuckGo.xcodeproj/project.pbxproj
#	DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants